[naga] Add GLSL constant_id layout qualifier support and WGSL backend override output#8589
Conversation
06wj
force-pushed
the
glsl-constant-id-support
branch
from
ef924a2 to
35ab6a0
Compare
06wj
force-pushed
the
glsl-constant-id-support
branch
2 times, most recently
from
916411a to
291adc7
Compare
naga/src/proc/constant_evaluator.rs
Outdated
| Behavior::Glsl(_) => { | ||
| unreachable!() | ||
| // GLSL specialization constants (constant_id) become Override expressions | ||
| Behavior::Glsl(GlslRestrictions::Const | GlslRestrictions::Runtime(_)) => { |
There was a problem hiding this comment.
So, GL_KHR_vulkan_glsl allows specialization constants to be used in constants:
layout(constant_id = 18) const int scX = 1;
layout(constant_id = 19) const int scZ = 1;
const ivec3 scVec = ivec3(scX, 1, scZ); // partially specialized vectorBut does this work, would you be able to add a test for this?
I think even if we allow overrides in GlslRestrictions::Const here it won't since the validator will complain that constants use overrides in their initializers.
The GLSL frontend would have to emit overrides for all constants that (indirectly) use specialization constants.
We can of course put this off and create an issue for it, but I was just wondering.
There was a problem hiding this comment.
Good catch! I've added the test case and you're right - it doesn't work correctly. The generated WGSL hardcodes the constant_id values (18, 19) instead of referencing the overrides.
const scVec: vec3<i32> = vec3<i32>(18i, 1i, 19i); Fixing this would require the GLSL frontend to track transitive dependencies on specialization constants. I think we should keep this PR focused on direct specialization constant usage (which works) and create a separate issue for this case.
Sound good?
There was a problem hiding this comment.
Yeah that sounds fine but it seems it's misbehaving rather than it being a validation issue, can you try doing this for the const path so that we get an error instead (for now):
Behavior::Glsl(GlslRestrictions::Const(_)) => {
Err(ConstantEvaluatorError::OverrideExpr)
}There was a problem hiding this comment.
Added the error message.
teoxoy
left a comment
teoxoy
left a comment
There was a problem hiding this comment.
I think this looks good! Just left a comment.
06wj
force-pushed
the
glsl-constant-id-support
branch
2 times, most recently
from
c78617b to
7f3c041
Compare
06wj
force-pushed
the
glsl-constant-id-support
branch
from
7f3c041 to
12e47af
Compare
2 participants
Connections
No dependencies on other PRs. Please add related issues if any.
Description
This PR adds support for GLSL specialization constants (
constant_idlayout qualifier) in the Naga GLSL frontend and enables the WGSL backend to outputoverridedeclarations.layout(constant_id = N)qualifier forconstdeclarationsOverrideinstead ofConstantOverrideexpressions in the constant evaluatorwrite_override()method to output@id(N) overridedeclarationsExpression::Overridein expression writerswrite_overrideinTypeContextNameKey::Overridevariant for override namingTesting
spec-constant.fragtest case with various specialization constant typesSquash or Rebase?
This branch contains a single functional commit and can be rebased directly.
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.